Skip to content

Implemeted SQL function center() that calculates the gravity center(centroid) from array of points #33

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

dura0ok
Copy link

@dura0ok dura0ok commented Jul 26, 2023

No description provided.

@dura0ok dura0ok changed the title Implemeted SQL function centroid() that calculates the gravity center… Implemeted SQL function center() that calculates the gravity center(centroid) from array of points Jul 26, 2023
@dura0ok dura0ok force-pushed the master branch 2 times, most recently from e08ff4d to 91174e7 Compare July 26, 2023 08:43
@dura0ok
Copy link
Author

dura0ok commented Jul 26, 2023

@esabol @vitcpp please, review it.

Makefile Outdated
@@ -31,7 +31,8 @@ DATA_built = $(RELEASE_SQL) \

DOCS = README.pg_sphere COPYRIGHT.pg_sphere
REGRESS = init tables points euler circle line ellipse poly path box index \
contains_ops contains_ops_compat bounding_box_gist gnomo epochprop
contains_ops contains_ops_compat bounding_box_gist gnomo epochprop\
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be a space between echoprop and the backslash on this line.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolved

@@ -393,7 +393,7 @@
The center operator <literal>@@</literal> is a non-boolean
unary operator returning the center of an object. In the
current implementation of <application>pgSphere</application>,
only centers of circles and ellipses are supported. Instead
only centers of circles, ellipses, point, array of points are supported. Instead
Copy link
Contributor

@esabol esabol Jul 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be points, arrays of points to be grammatically correct here. I think this line is too long, and it needs to be word-wrapped after are, along with the next couple of lines where needed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolved

@@ -167,3 +167,33 @@ CREATE OPERATOR <-> (
COMMENT ON OPERATOR <-> (spoint, spoint) IS
'distance between spherical points';

CREATE FUNCTION center(dots_vector SPoint[])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we had settled on naming this function centroid? Why was center chosen?

I see it referred to as "centroid" at these URLs:

https://stackoverflow.com/a/38201499/5153779
https://gis.stackexchange.com/questions/43505/calculating-a-spherical-polygon-centroid

However, "center" is used here:
https://github.com/chrisveness/geodesy/blob/8f4ef33d/latlon-nvector-spherical.js#L783

So I'm not sure which term is correct.

Does your implementation give the same results as the Python and JavaScript implementations at https://stackoverflow.com/questions/19897187#answer-38201499 and https://github.com/chrisveness/geodesy/blob/8f4ef33d/latlon-nvector-spherical.js#L783 ? If not, why not?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vitcpp and I discussed the name "centroid" earlier, but in the end we came to the conclusion that the best name is the center for the sql function to match the interface with the operator @@

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@esabol The main reason to choose 'center' function because such function already exists. It returns the center of circle or ellipse and it is intended to be expanded to other object types. In the doc it is described as center of an object. It was decided to overload this function and allow to pass array of points and the point itself (for interface completeness). It is why we decided to put on display PR with 'center' function. Furthermore, we may utilize operator @@. My opinion is that center and centroid are interchangeable in most cases. So, our idea is to reuse center and to avoid introducing new terms.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, that makes sense (the center name) then, but I'd still like an answer to this:

Does your implementation give the same results as the Python and JavaScript implementations at https://stackoverflow.com/questions/19897187#answer-38201499 and https://github.com/chrisveness/geodesy/blob/8f4ef33d/latlon-nvector-spherical.js#L783 ? If not, why not?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@esabol But this is function, which

Calculates the centre of a spherical polygon where the sides of the polygon are great circle arcs joining the vertices.
In my task, I have an arbitrary set of points. That is why I have now tested and on any two input points their function gives 0, because the polygon starts with three points

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@esabol Sorry... Give me time, then I'll try to figure out why the answers are completely different

Copy link
Contributor

@vitcpp vitcpp Jul 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dear Colleagues, it was intended that the proposed center function calculates the mass center of a set of points (particles) that do not form a figure. Such function may be useful in some astronomical tasks when we have to deal with stellar clusters or with other groups of astronomical objects which mass center on the sphere we'd like to find. When we want to find the center of a figure we definitely should use the Stokes theorem as it was implemented in the referenced JS library (center(spoly) should be implemented that way). I guess it is why @stepan-neretin7 sees some differences when trying to compare the proposed function against the function to calculate polygon center from the referenced JS library. Adding some more points on a polygon edge doesn't change the shape of the polygon. But adding more points change the mass center of the set of particles (points). There is another question - is it reasonable to implement such function? I think yes. Probably, the name center is best suited to calculate the center of a figure than to calculate the mass center of a set of points (may be come back to centroid - ?). I'm not sure about having such similar function in the JS library. If yes, it is reasonable to compare our function against its counterpart.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK with me.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vitcpp @esabol
Dear colleagues, I want to revive the discussion of my pull-request. I have two questions that I would like to discuss? First; After all, we'd better leave such an implementation (the center of an arbitrary set of points, not a polygon.. If we add more points on the side of an imaginary polygon from points, then my implementation will shift to this side) or we will think in the direction of that implementation for polygons through the Stokes theorem
Second: When two points are at the poles: point(0,-pi()/2), point(0,pi()/2), then the middle point will be ~epsilon and it is incorrect to translate this into lat, lng, because such a point is not projected onto the sphere. What should I give out in this case? NULL? But let's say the case with the points 0,0 and 0, pi
The center here is any point on a large circle perpendicular to the line connecting the points
What is better to return here? NAN? NULL?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deal All, the final result of this operation is to find centroid of a set of points, not the center of a polygon. We do not find the center of a figure like polygon in this task. The averaged vector is pretty fine solution. There are two special cases. Case 1: we find the center of two opposite points. In this case, the equidistant place is a circle, not a point. The geometric
center in the Cartesian coordinate system is the center of the sphere. Case 2: when we have, for example, two points on poles and 4 points on equator with longitudes 0, 90, 180, 270 degrees. In this case the geometric center in 3D is (0,0,0) but there is no geometric center on the sphere. Case 3: we have all points on the same big circle. In this case, there are two points are equidistant. I'm not sure about other special cases. I guess, there are no other special cases.

This function may be useful to find the approximate center of a group of astronomical objects that are placed in a local area on the sphere: the center of a star cluster, the center of a selected group of objects, etc. I think the defined special cases are impractical; I'm not sure in what tasks it can happen.

I propose this function to return NULL if 3D geometric center is (0,0,0) +- EPSILON. It means, the algorithm can't find centroid of the point set. I think, NULL is better because one can use ifull or coalesce to define some result in such cases.

@vitcpp
Copy link
Contributor

vitcpp commented Jul 29, 2023

I have some concerns related to the cases when cartesian geometric center lies in the center of the sphere. The following situations are possible:

  • The center on the sphere is absent. I guess in this case the function returns (0,0).

  • There are two possible centers on the sphere when two points are locates on the opposite sides of a meridian. But in this case due to rounding issues in most situations out calculated center in 3D will be near the center of the sphere but not exactly there. It affects which point will be choosen as the result and may produce unstable results I guess.

@dura0ok
Copy link
Author

dura0ok commented Aug 5, 2023

Other popular libraries, except javascript, do not even allow you to check which center of mass they give out for two antipodal points, because they write that a minimum of 6 points is needed to create a polygon. And for some reason the Javascript library writes (0,0), which is fundamentally wrong

@vitcpp
Copy link
Contributor

vitcpp commented Aug 5, 2023

We can't compare the function for calculation the centroid of a set of independent points with the calculation of a polygon figure. The set of independent points does not form a figure. We should compare the equivalent function in other spherical geometry libraries if such function exist. The only unresolved question - what to return when the points are on opposite sides. There are two cases: either we have two result points or there are no points at all (when the calculated center in 3D is near the center of the sphere).

@vitcpp
Copy link
Contributor

vitcpp commented Dec 22, 2023

There are some conceptual problems with centroid function. I propose to close it as it is not required right now. Later we may reopen it if neeed.

@vitcpp
Copy link
Contributor

vitcpp commented Dec 27, 2023

Closed as incomplete.

@vitcpp vitcpp closed this Dec 27, 2023
@esabol esabol mentioned this pull request Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants